INTERNAL: add PipeOperationImpl to reduce duplicate code #872

oliviarla · 2025-01-22T02:23:40Z

🔗 Related Issue

앞으로 pipe operation implementation 변경 지점이 많아질 것을 고려하여 PipeOperationImpl 클래스를 추가하여 이를 각각의 클래스가 상속받도록 합니다.
또한, 이전에 pipe_error까지 읽는 로직을 CollectionPipedInsertOperationImpl에만 적용했었기 때문에, 하나의 추상 클래스를 상속받도록 하여 CollectionPipedUpdateOperationImpl, CollectionPipedExistOperationImpl, CollectionBulkInsertOperationImpl에도 적용하고자 합니다.

⌨️ What I did

handleLine, initialize 등의 메서드 중복과 static 필드 중복을 제거하였습니다.
handleLine에서 각 연산(insert/exist/update/bulk insert)마다 다르게 처리해야 하는 부분은 추상 메서드로 빼 각 클래스에서 구현하도록 하였습니다.
wasCancelled 메서드는 연산마다 다룰 CANCELLED 상태가 달라 각 클래스에서 구현하도록 하였습니다.
getInsert/ getExist / getUpdate 메서드는 형변환이 필요하기 때문에 각 클래스에서 구현하도록 하였습니다.

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java

jhpark816

리뷰 완료

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java

oliviarla · 2025-01-24T02:52:53Z

@uhm0311 @jhpark816 다시 리뷰 부탁드립니다.

jhpark816

일부 리뷰

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java

jhpark816

추가 리뷰

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java

jhpark816 · 2025-01-24T09:00:20Z

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java

+        } else {
+          index++;
+        }
+      }


질문입니다.
isNotPiped() 경우에서 NOT_MY_KEY 응답을 받으면,
single key pipe이든 multi key pipe이든 관계없이 아래 로직을 사용할 수 있나요?

addRedirectSingleKeyOperation(line, keys.get(index)); transitionState(OperationState.REDIRECT);

제시하신 로직을 사용해도 정상 동작할 것으로 보입니다.

isNotPiped인 경우가 발생하려면 1. 처음부터 노드에 보낼 연산의 키가 1개뿐이거나, 2. 맨 마지막 연산만 빼고 이미 수행된 상태에서 switchover/redirect로 인해 다시 요청을 보냈을 때 뿐입니다.

1번 케이스

내부에 키가 하나뿐인 single key redirect handler를 생성하여, 리다이렉트할 캐시 노드를 찾고 해당 키에 대해서만 요청을 보냅니다.

2번 케이스

기존에 NOT_MY_KEY 응답을 받았다면 이미 내부에 키가 하나 이상 존재하는 multi key redirect handler가 생성된 상태입니다. 따라서 이 핸들러에 단순히 키만 추가하게 되어 이후 동작에 문제가 없습니다.

redirect handler가 존재하지 않았다면 내부에 키가 하나뿐인 single key redirect handler를 생성하여, 리다이렉트할 캐시 노드를 찾고 해당 키에 대해서만 요청을 보냅니다.

oliviarla requested review from jhpark816, uhm0311 and brido4125 January 22, 2025 02:23

oliviarla self-assigned this Jan 22, 2025

oliviarla force-pushed the pipeend branch from 1e3c32e to a5a1d2b Compare January 22, 2025 02:26

brido4125 requested changes Jan 22, 2025

View reviewed changes

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java Show resolved Hide resolved

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java Show resolved Hide resolved

jhpark816 reviewed Jan 22, 2025

View reviewed changes

oliviarla force-pushed the pipeend branch from a5a1d2b to e42add7 Compare January 23, 2025 02:51

oliviarla mentioned this pull request Jan 23, 2025

INTERNAL: integrate piped operation callbacks #873

Merged

oliviarla force-pushed the pipeend branch from e42add7 to 2dc627d Compare January 24, 2025 02:30

oliviarla marked this pull request as draft January 24, 2025 02:30

oliviarla force-pushed the pipeend branch from 2dc627d to 4e7e704 Compare January 24, 2025 02:41

oliviarla marked this pull request as ready for review January 24, 2025 02:41

oliviarla force-pushed the pipeend branch from 4e7e704 to f68e791 Compare January 24, 2025 02:42

oliviarla force-pushed the pipeend branch from f68e791 to fa1c54b Compare January 24, 2025 06:02

jhpark816 reviewed Jan 24, 2025

View reviewed changes

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java Outdated Show resolved Hide resolved

src/main/java/net/spy/memcached/protocol/ascii/PipeOperationImpl.java Outdated Show resolved Hide resolved

jhpark816 reviewed Jan 24, 2025

View reviewed changes

oliviarla force-pushed the pipeend branch from fa1c54b to 0a567fe Compare January 31, 2025 02:14

INTERNAL: add PipeOperationImpl to reduce duplicate code

845df10

oliviarla force-pushed the pipeend branch from 0a567fe to 845df10 Compare January 31, 2025 02:16

jhpark816 approved these changes Jan 31, 2025

View reviewed changes

jhpark816 requested a review from brido4125 January 31, 2025 03:10

jhpark816 merged commit 451644c into naver:develop Jan 31, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INTERNAL: add PipeOperationImpl to reduce duplicate code #872

INTERNAL: add PipeOperationImpl to reduce duplicate code #872

oliviarla commented Jan 22, 2025 •

edited

Loading

jhpark816 left a comment

oliviarla commented Jan 24, 2025

jhpark816 left a comment

jhpark816 left a comment

jhpark816 Jan 24, 2025

oliviarla Jan 31, 2025

INTERNAL: add PipeOperationImpl to reduce duplicate code #872

INTERNAL: add PipeOperationImpl to reduce duplicate code #872

Conversation

oliviarla commented Jan 22, 2025 • edited Loading

🔗 Related Issue

⌨️ What I did

jhpark816 left a comment

Choose a reason for hiding this comment

oliviarla commented Jan 24, 2025

jhpark816 left a comment

Choose a reason for hiding this comment

jhpark816 left a comment

Choose a reason for hiding this comment

jhpark816 Jan 24, 2025

Choose a reason for hiding this comment

oliviarla Jan 31, 2025

Choose a reason for hiding this comment

oliviarla commented Jan 22, 2025 •

edited

Loading